-
Notifications
You must be signed in to change notification settings - Fork 204
feat: Add support for MCP Bundles (MCPB) in registry #295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Add support for MCP Bundles (MCPB) in registry #295
Conversation
- Add MCPB as a new registry_name option - Add file_hashes field to Package model for integrity verification - Implement URL validation restricting MCPB packages to GitHub/GitLab hosts - Require SHA-256 hash for all MCPB packages - Add example MCPB package in documentation - Update schemas and OpenAPI specification Implements Option 1 from issue modelcontextprotocol#260 - MCPB as an additional package type pointing to hosted .mcpb files with cryptographic verification.
} | ||
|
||
// Allowlist of trusted hosts for MCPB packages | ||
allowedHosts := []string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to verify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
allowedHosts := []string{ | ||
"github.com", | ||
"www.github.com", | ||
"raw.githubusercontent.com", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly this one (raw.githubusercontent.com) is not necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little uneasy about shoehorning registry_name
and name
to this purpose, while we still have the opportunity to make pre-release breaking changes. Thoughts on the proposed clearer modeling?
docs/server-json/examples.md
Outdated
"packages": [ | ||
{ | ||
"registry_name": "mcpb", | ||
"name": "https://github.com/modelcontextprotocol/text-editor-mcpb/releases/download/v1.0.2/text-editor.mcpb", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is right with our current schema. But a thought for potentially renaming some details here: name
doesn't really make sense for a URL like this, and registry_name
doesn't for mcpb
either (it's more of a package type
).
I think I'd be in favor of something more like this here:
{
"location": {
"type": "mcpb",
"url": "https://github.com/modelcontextprotocol/text-editor-mcpb/releases/download/v1.0.2/text-editor.mcpb"
},
"runtime_hint": ...
...
}
And that's it - drop version
, because the only reason we have version
is to effectively "construct" the url
in the registry_name
case.
Basically, my proposal is to add a nested PackageLocation
object inside here, where we currently just store package_name, name, and version in the top level of the Package
object. It would be an "OR" situation where either the original fields (registry_name, name, and version) or the new fields (type, url) are required. mcpb
is the only valid entry for type
.
Then the original case for other examples, like the one just before this, becomes:
"packages": [
{
"location": {
"registry_name": "npm",
"name": "@example/hybrid-mcp-server",
"version": "1.5.0"
},
"runtime_hint": "npx",
"package_arguments": [
{
"type": "named",
"name": "--mode",
"description": "Operation mode",
"default": "local",
"choices": ["local", "cached", "proxy"]
}
]
}
]
It's a breaking change but I think at this point, pre-release, is a good time to make this rather than shoehorn values into registry_name
and name
. What do you all think? cc @domdomegg @joan-anthropic @toby
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tadasant thanks for the quick turnaround. Agree that registry_name
and name
feel a bit overloaded in the current proposal - I like the distinction.
It'd be nice to keep version as a separate field - to better enable future auto-update workflows for DXTs. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow the use case, could you explain auto-update workflows for DXTs? Is there a reason you wouldn't be able to use the https://github.com/modelcontextprotocol/text-editor-mcpb/releases/download/v1.0.2/text-editor.mcpb
value for that (which has the version in it)?
We've had some feedback from @toby that all these version
fields throughout the various server.json levels are causing confusion, so my thinking is that this is a good opportunity to potentially eliminate one of them as it's not actually necessary (but maybe I'm missing something).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Maybe package_type and package_identifier seems good? From question on the docker runtime type #249 (reply in thread). I think also as a meta thing I'd prefer them to stay in the package object rather than a nested location object, because I think unnecessary nesting is painful for many ecosystems (e.g. parsing nested structs like this in Java creates a lot of boilerplate).
So together I think I might propose something like:
"packages": [
{
"package_type": "mcpb",
"package_identifier": "https://github.com/modelcontextprotocol/cool-mcp/releases/download/v1.2.3/cool-mcp.mcpb",
// there's something that aesthetically feels a little worse about this not being an object, but I think it will make it easier on consumers. and very unlikely to really need other hash functions unless sha256 is broken
"package_hash_sha256": "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce"
},
{
"package_type": "docker",
// docker implicitly has registry name in package_identifier, I think we should just have package_identifier because of this
"package_identifier": "ghcr.io/domdomegg/cool-mcp:1.2.3@sha256:4a7f307d128da53e99ea7f45064df440c742aa3f648a09a09b51dfd1cd55378b"
},
{
"package_type": "commonjs",
// i think for now only allowlist npmjs. but subregistries could use e.g. https://npm.pkg.github.com or https://artifactory.corp.internal/npm
"registry_name": "https://registry.npmjs.org/",
"package_identifier": "@domdomegg/cool-mcp@1.2.3",
"runtime_hint": "npx",
"package_arguments": [
{
"type": "named",
"name": "--mode",
"description": "Operation mode",
"default": "local",
"choices": ["local", "cached", "proxy"]
}
]
}
]
- I think I agree with @tadasant that the version is not necessary in the
packages
array for DXT. There will still be a version at the top level in server.json. For auto-update I think we'd use this top-level version, and maybeis_latest
.
"file_hashes": { | ||
"sha-256": "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider lifting this inside the PackageLocation
object I am proposing above, because it is only a concern relevant to non-package-registry entries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess there is the possibility we add some registry in the future that does not do its own checks. So I'm fine keeping it here as an optional field.
I assume we need to rely on the MCP Registry to generate this file hash at publish-time, right? Or is the idea it will get generated by the CLI tool en route to creating a server.json, and the registry just validates it's correct and not tampered with in the publish flow?
I think we should map out and document this feature in a little more detail, maybe a dedicated doc in docs/
explaining the flows, who's responsible for what. I worry a bit about how file_hashes
is something we are expecting in the immutable server.json file, but also need to verify they're actually valid hashes. Seems doable but would like to see a step by step explanation somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not opinionated here and will defer to you all re: what makes sense for the workflow the registry committee would prefer to support; happy add docs to reflect.
- Add hash generation during publish (can be disabled with --no-hash) - Add verify command to check existing hashes - Add hash-gen command to generate/update hashes - Support NPM and direct URL (MCPB) packages - Document implementation in docs/file-hashes.md The CLI tool now generates SHA-256 hashes at publish time. Registry validates these hashes to ensure package integrity.
@joan-anthropic another thought just came to me (sorry for the thrashing): what if instead of For example, this old format: {
"registry_name": "npm",
"name": "@example/hybrid-mcp-server",
"version": "1.5.0",
"runtime_hint": "npx",
"package_arguments": [
{
"type": "named",
"name": "--mode",
"description": "Operation mode",
"default": "local",
"choices": ["local", "cached", "proxy"]
}
]
} Becomes: {
"url": "https://www.npmjs.com/package/@example/hybrid-mcp-server/v/1.5.0",
"type": "javascript",
"runtime_hint": "npx",
"package_arguments": [
{
"type": "named",
"name": "--mode",
"description": "Operation mode",
"default": "local",
"choices": ["local", "cached", "proxy"]
}
]
} So then we have just one way of doing things. We can still properly sanitize and logic branch on Running out of time here today on my end to fully think through this, but this seems like a potentially cleaner approach here and drawbacks aren't coming to top of mind. I also like how it helps us remove the schema-level dependency on enumerating all these branded centralized registry names Curious for @domdomegg's take here |
Yep I like this! I ended up commenting something similar before I scrolled far enough down to see this. I think I prefer the direction of this approach. I don't mind too much between a URL and slightly more structure (e.g. package type, registry name, identifier). I think I have a weak preference towards the slightly more structure just because I think clients might have an easier time parsing it to programmatically get packages etc. But fine to go with URL, especially if we provide guidance on the formats of URLs. Perhaps also some small SDKs in common languages for parsing and running server.jsons, as I imagine clients will want something like that anyway. |
Motivation and Context
See issue #260
How Has This Been Tested?
Breaking Changes
N/A, registry has not yet been launched
Types of changes
Checklist
Additional context